Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: api integration tests #133

Merged
merged 38 commits into from
Aug 6, 2024
Merged

refactor: api integration tests #133

merged 38 commits into from
Aug 6, 2024

Conversation

Daanvdplas
Copy link
Collaborator

@Daanvdplas Daanvdplas commented Jul 26, 2024 •
edited
Loading

Closes #123 & #120

  • Remove helper functions to the bottom of the file
  • improved return value when calling the contracts so we can assert on a success and error at the same time.
  • Have a consistent assert schema: value, expected
  • if a contract is reverted it is clear from which function it is coming from.

Open question that might improve things even more:
https://substrate.stackexchange.com/questions/11765/what-should-data-be-decoded-to-in-execreturnvalue

Daanvdplas and others added 30 commits July 19, 2024 18:26
@Daanvdplas
feat: pop api fungibles pallet
360bb2a
@Daanvdplas
refactor: comments
e097f80
@evilrobot-01 @Daanvdplas
refactor: rebase main, refactor extension with enums
c8d8b2a
@Daanvdplas
refactor
c88387d
@Daanvdplas
refactor 2
224390a
@Daanvdplas
feat: add fungibles pallet functions
43ae445
@Daanvdplas
tests: remove transfer_from (transfer copy) and add approve
bbe01a1
@Daanvdplas
refactor: primitives
68264da
@Daanvdplas
fix: pallet api error + missing docs
2481773
@Daanvdplas
fix: integration test after removal fungibles feature primitives
c10a8d4
@Daanvdplas
chore: benchmark approve
a8c4bb5
@Daanvdplas
refactor: pallet
4419bad
@Daanvdplas
refactor: integration-tests
792fa07
@Daanvdplas
refactor: ink api library
96da2f0
@Daanvdplas
refactor: devnet runtime
cb54a57
@Daanvdplas
chore: benchmark fungibles approve
9ec2744
@Daanvdplas
test: ensure Lookup configuration
f0fff97
@Daanvdplas
fix: approve benchmark + weights
4a476b0
@Daanvdplas
refactor: primitives
f3cc83a
@Daanvdplas
refactor: bits and pieces
bd85489
@Daanvdplas
fix: test devnet
0e49f63
@Daanvdplas
refactor: apply final comments
8f6139c
@Daanvdplas
chore: benchmark approve
948e144
@Daanvdplas
refactor: read state
88577e4
@Daanvdplas
refactor: add state query filter
809e192
@Daanvdplas
fix: test UnknownCall
ac9714c
@Daanvdplas
refactor: read state functions
77711ec
@Daanvdplas
refactor: enums
b50830c
@Daanvdplas
refactor: style docs and using constants for errors in chain extension
2a8800c
@Daanvdplas
refactor: api integration tests
b93f009
@evilrobot-01
Copy link
Collaborator

The link you included on SE is broken.

Base automatically changed from daan/refactor-remove_read_state_encoding to daan/api July 28, 2024 10:07
@Daanvdplas Daanvdplas closed this Jul 30, 2024
@Daanvdplas Daanvdplas reopened this Jul 30, 2024
@Daanvdplas Daanvdplas closed this Jul 31, 2024
@Daanvdplas Daanvdplas reopened this Jul 31, 2024
peterwht
peterwht reviewed Aug 2, 2024
View reviewed changes
Copy link
Collaborator

@peterwht peterwht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, small comments to resolve.

GitHub is not letting me put a comment on it. But the if-chain on line 156 has a clippy warning.

Please resolve it.

match value.cmp(current_allowance) =>
    Equal => {...},
    Less => {...},
    Greater => {...},

pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
@Daanvdplas Daanvdplas merged commit d8ac5bd into daan/api Aug 6, 2024
6 checks passed
@Daanvdplas Daanvdplas deleted the daan/fix-read_state_error_handling branch August 6, 2024 12:45
This was referenced Aug 6, 2024
Closed
Closed
chungquantin pushed a commit that referenced this pull request Sep 6, 2024
@Daanvdplas @evilrobot-01 @chungquantin
refactor: api integration tests (#133)
f2f35ca 
Co-authored-by: Frank Bell <frank@r0gue.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterwht peterwht peterwht left review comments

@chungquantin chungquantin chungquantin approved these changes

@al3mart al3mart Awaiting requested review from al3mart

@evilrobot-01 evilrobot-01 Awaiting requested review from evilrobot-01

Assignees
No one assigned
Labels
api Pop API
Projects
None yet
Milestone
No milestone
4 participants
@Daanvdplas @evilrobot-01 @peterwht @chungquantin